feat (846): added ledger object hashing functionality #862
Conversation
WalkthroughAdds a new xrpl.utils.hash_utils module implementing ledger-object hashing utilities (AccountRoot, Offer/OfferID, Check, Escrow, PaymentChannel, SignerList, Trustline/RippleState, Ticket, DepositPreauth), re-exports them via xrpl.utils.init, adds unit tests, and updates CHANGELOG.md documenting the additions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant HashUtils as hash_utils
participant AddrCodec as addresscodec
participant SHA as "SHA-512\n(half)"
Caller->>HashUtils: call hash_* (address, seq, currency...)
HashUtils->>AddrCodec: normalize/encode addresses
AddrCodec-->>HashUtils: address hex (20 bytes)
HashUtils->>HashUtils: ledger_space_hex(name) & normalize components
HashUtils->>SHA: sha512_half(concatenated hex payload)
SHA-->>HashUtils: 32-byte digest (64 hex chars)
HashUtils-->>Caller: uppercase 64-char hex hash
rect rgba(220,240,255,0.6)
note right of HashUtils: Trustline flow orders addresses deterministically\nand normalizes currency (3-letter / bytes / hex)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
xrpl/utils/hash_utils.py (5)
19-22: Remove or use unused constants.
HEXandBYTE_LENGTHare unused. Either remove them or useBYTE_LENGTHto derive the8hex padding for 32‑bit numbers.-HEX = 16 -BYTE_LENGTH = 4 # 4 bytes for sequence numbers +BYTE_LENGTH = 4 # 4 bytes for sequence numbers +SEQ_HEX_LEN = BYTE_LENGTH * 2 # 8 hex chars
59-72: Docstring: broaden exception type.
decode_classic_addresscan raiseValueErrorin addition toXRPLAddressCodecException. Reflect this in the “Raises” section.- Raises: - XRPLAddressCodecException: If the address is invalid. + Raises: + XRPLAddressCodecException or ValueError: If the address is invalid.
101-116: DRY up sequence formatting and validate u32 range.Several functions repeat
format(seq, "x").zfill(8). Extract a helper that validates 0 ≤ n ≤ 2^32−1 and formats once.+def _u32_hex(n: int) -> str: + if n < 0 or n > 0xFFFFFFFF: + raise ValueError("Sequence must be in range [0, 2^32 - 1].") + return format(n, "x").zfill(8) @@ - + format(sequence, "x").zfill(8) # 4 bytes = 8 hex characters + + _u32_hex(sequence) @@ - + format(sequence, "x").zfill(8) # 4 bytes = 8 hex characters + + _u32_hex(sequence) @@ - + format(sequence, "x").zfill(8) # 4 bytes = 8 hex characters + + _u32_hex(sequence) @@ - + format(ticket_id, "x").zfill(8) # 4 bytes = 8 hex characters + + _u32_hex(ticket_id)Also applies to: 118-133, 135-150, 152-169, 220-235
171-183: Avoid magic “00000000”.Use the shared helper for the zero list ID or a named constant for readability.
- ledger_space_hex("signerList") + address_to_hex(address) + "00000000" + ledger_space_hex("signerList") + address_to_hex(address) + _u32_hex(0)
237-251: URIToken note.URIToken is Xahau-specific. The helper is fine, but consider guarding docs to avoid implying XRPL-mainnet support.
CHANGELOG.md (1)
10-15: Tighten wording; list aliases for completeness.Minor copy tweak and add alias names for discoverability.
-### Added -- Added ledger object hashing utilities for computing hashes of various XRPL ledger objects -- Added `hash_account_root`, `hash_offer`, `hash_escrow`, `hash_payment_channel`, `hash_trustline`, `hash_signer_list_id`, `hash_check`, `hash_ticket`, `hash_deposit_preauth`, and `hash_uri_token` functions -- Added comprehensive test coverage for all hash utility functions -- These utilities are essential for Batch transactions where you need to know the hash of a ledger entry object before it's created on the ledger +### Added +- Ledger object hashing utilities for XRPL ledger objects. +- New functions: `hash_account_root`, `hash_offer` (alias: `hash_offer_id`), `hash_escrow`, `hash_payment_channel`, `hash_trustline` (alias: `hash_ripple_state`), `hash_signer_list_id`, `hash_check`, `hash_ticket`, `hash_deposit_preauth`, `hash_uri_token`. +- Comprehensive unit tests for all hash utilities. +- Useful for Batch transactions that require the ledger entry hash prior to creation.tests/unit/utils/test_hash_utils.py (2)
148-155: Fix E128: align continuation line.Indent the second argument under the opening parenthesis.
- self.assertEqual(hash_trustline(address1, address2, currency), - hash_ripple_state(address1, address2, currency)) + self.assertEqual( + hash_trustline(address1, address2, currency), + hash_ripple_state(address1, address2, currency), + )
250-275: Add case-insensitivity test for 3-letter currencies.Verifies the uppercase normalization in
hash_trustline.@@ def test_currency_formats(self): """Test different currency formats in trustline hash.""" address1 = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh" address2 = "rB5TihdPbKgMrkFqrqUC3yLdE8hhv4BdeY" @@ result_eur = hash_trustline(address1, address2, "EUR") self.assertEqual(len(result_eur), 64) self.assertNotEqual(result_usd, result_eur) + + # Lowercase 3-letter currency should hash the same after normalization + result_usd_lower = hash_trustline(address1, address2, "usd") + self.assertEqual(result_usd, result_usd_lower)xrpl/utils/__init__.py (1)
52-64: all exports look correct; minor docs/ordering nit.
- Keeping both canonical names and aliases (hash_trustline/hash_ripple_state, hash_offer/hash_offer_id) is fine; ensure docs show the canonical once and mention aliases to avoid duplicate entries.
- Maintain alphabetical order within this group for future additions to reduce churn (current block already appears alphabetized).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)tests/unit/utils/test_hash_utils.py(1 hunks)xrpl/utils/__init__.py(2 hunks)xrpl/utils/hash_utils.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/unit/utils/test_hash_utils.py (2)
xrpl/core/addresscodec/exceptions.py (1)
XRPLAddressCodecException(6-9)xrpl/utils/hash_utils.py (12)
address_to_hex(59-71)hash_account_root(89-98)hash_check(118-132)hash_escrow(135-149)hash_offer(101-115)hash_payment_channel(152-168)hash_signer_list_id(171-182)hash_ticket(220-234)hash_trustline(185-217)hash_uri_token(237-251)ledger_space_hex(74-86)sha512_half(46-56)
xrpl/utils/hash_utils.py (2)
xrpl/utils/str_conversions.py (1)
str_to_hex(4-16)xrpl/core/addresscodec/codec.py (1)
decode_classic_address(146-156)
xrpl/utils/__init__.py (1)
xrpl/utils/hash_utils.py (9)
hash_account_root(89-98)hash_check(118-132)hash_escrow(135-149)hash_offer(101-115)hash_payment_channel(152-168)hash_signer_list_id(171-182)hash_ticket(220-234)hash_trustline(185-217)hash_uri_token(237-251)
🪛 Flake8 (7.2.0)
tests/unit/utils/test_hash_utils.py
[error] 154-154: continuation line under-indented for visual indent
(E128)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~10-~10: There might be a mistake here.
Context: ...0.0.html). ## [[Unreleased]] ### Added - Added ledger object hashing utilities fo...
(QB_NEW_EN)
🔇 Additional comments (8)
xrpl/utils/hash_utils.py (6)
24-43: Confirm ledger space codes (esp. uriToken).The mapping looks consistent with rippled/xrpl.js. Please confirm
uriToken: 'U'matches the current upstream standard and networks you target (XRPL vs Xahau).
46-57: SHA-512Half implementation LGTM.Correctly converts hex→bytes, hashes, returns first 32 bytes as 64 hex chars (uppercased).
74-87: ledger_space_hex implementation LGTM.Two‑byte ASCII code, zero‑padded to 4 hex characters; raises
KeyErrorfor unknown spaces.
152-169: Payment channel address order.Spec requires creator address first, then destination, then sequence. Your order matches that, so no change; just calling it out.
254-268: Remaining helpers and aliases LGTM.Consistent with rippled keylet derivations; aliases help with discoverability.
Also applies to: 270-272
185-217: Preserve case-sensitivity of 3-char currency codes; remove uppercase normalization and hex-length enforcement.Currency codes are case-sensitive per XRPL spec and xrpl.js preserves the original case when encoding (xrpl.org, github.com). ripple-binary-codec does not normalize codes to uppercase nor validate non-standard hex length.
Likely an incorrect or invalid review comment.
tests/unit/utils/test_hash_utils.py (1)
1-323: Overall tests LGTM.Good coverage, vectors, aliases, ordering, edge cases, and output invariants.
xrpl/utils/__init__.py (1)
5-18: All hash utilities correctly re-exported; no circular imports detected. All imported symbols exist inxrpl/utils/hash_utils.py, and there are no top-level imports ofxrpl.utilsthat could introduce circularity.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
CHANGELOG.md (1)
10-15: Tighten language and align with changelog style.
- Replace “prior to creation” with “before creation” for brevity.
- Optional: add a brief note that outputs are 64‑char uppercase hex to set expectations.
Apply:
- - Useful for Batch transactions that require the ledger entry hash prior to creation. + - Useful for Batch transactions that require the ledger entry hash before creation. + - All functions return 64-character uppercase hex (SHA-512Half).tests/unit/utils/test_hash_utils.py (4)
147-154: Fix E128 (under‑indented continuation) for readability/CI.Break arguments onto separate lines.
- self.assertEqual(hash_trustline(address1, address2, currency), - hash_ripple_state(address1, address2, currency)) + self.assertEqual( + hash_trustline(address1, address2, currency), + hash_ripple_state(address1, address2, currency), + )
252-284: Add negative tests for invalid currency encodings (trustline).Currently only valid cases are tested. Add cases to assert ValueError on:
- bytes length != 20
- hex string not 40 hex chars or with non-hex chars
I can draft the exact tests once implementation enforces these validations.
177-187: Assert directionality for DepositPreauth.Add a test showing that swapping address and authorized_address yields a different hash (the object is directional).
28-37: Strengthen sha512_half test with a fixed vector.Consider asserting the exact expected digest for “Hello World” hex to catch regressions, not just length/case.
xrpl/utils/hash_utils.py (1)
57-72: Ruff TRY003 and message clarity.Shorten/standardize the error message; optionally move to a constant. Functional behavior unchanged.
- if n < 0 or n > 0xFFFFFFFF: - raise ValueError("Sequence must be in range [0, 2^32 - 1].") + if n < 0 or n > 0xFFFFFFFF: + raise ValueError("u32 sequence out of range (0..=2^32-1)")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
CHANGELOG.md(1 hunks)tests/unit/utils/test_hash_utils.py(1 hunks)xrpl/utils/__init__.py(2 hunks)xrpl/utils/hash_utils.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- xrpl/utils/init.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/unit/utils/test_hash_utils.py (2)
xrpl/core/addresscodec/exceptions.py (1)
XRPLAddressCodecException(6-9)xrpl/utils/hash_utils.py (11)
address_to_hex(74-86)hash_account_root(104-113)hash_check(133-147)hash_escrow(150-164)hash_offer(116-130)hash_payment_channel(167-183)hash_signer_list_id(186-197)hash_ticket(235-249)hash_trustline(200-232)ledger_space_hex(89-101)sha512_half(44-54)
xrpl/utils/hash_utils.py (1)
xrpl/core/addresscodec/codec.py (1)
decode_classic_address(146-156)
🪛 Flake8 (7.2.0)
tests/unit/utils/test_hash_utils.py
[error] 153-153: continuation line under-indented for visual indent
(E128)
🪛 Ruff (0.12.2)
xrpl/utils/hash_utils.py
70-70: Avoid specifying long messages outside the exception class
(TRY003)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...shing utilities for XRPL ledger objects. - New functions: hash_account_root, `has...
(QB_NEW_EN)
[style] ~14-~14: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...ions that require the ledger entry hash prior to creation. ### Fixed - Removed snippets...
(EN_WORDINESS_PREMIUM_PRIOR_TO)
🔇 Additional comments (3)
CHANGELOG.md (1)
10-13: Public API exports for hashing utilities are correct. All new hash functions and aliases match the CHANGELOG and no extra exports (e.g.hash_uri_token) are present.xrpl/utils/hash_utils.py (2)
89-102: LGTM: ledger-space encoding matches XRPL keylet conventions.Zero-padded 16‑bit namespace from ASCII code is correct (e.g., account→0061).
186-197: LGTM: SignerList index formula.Namespace ‘S’ + AccountID + 0x00000000 matches rippled behavior.
… currency formats
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/unit/utils/test_hash_utils.py (1)
117-128: Add an explicit payment-channel vector assertion (cross-lib parity).Current test only checks shape. Add a known-good expected hash from xrpl.js/rippled for these inputs to guard against regressions.
def test_hash_payment_channel(self): """Test payment channel hash calculation.""" # Test with valid addresses address = "rDx69ebzbowuqztksVDmZXjizTd12BVr4x" dst_address = "rLFtVprxUEfsH54eCWKsZrEQzMDsx1wqso" sequence = 82 - result = hash_payment_channel(address, dst_address, sequence) + result = hash_payment_channel(address, dst_address, sequence) # Should return a 64-character uppercase hex string self.assertEqual(len(result), 64) self.assertTrue(result.isupper()) self.assertTrue(all(c in "0123456789ABCDEF" for c in result)) + # Known vector from xrpl.js tests + expected = "<fill-with-known-expected-from-xrpl.js>" + # Uncomment once expected is populated + # self.assertEqual(result, expected)
🧹 Nitpick comments (7)
xrpl/utils/hash_utils.py (5)
45-56: Accept bytes in sha512_half for ergonomic and zero-copy usage.Allow bytes/bytearray/memoryview in addition to hex str to avoid pre-encoding and make the function broadly reusable. Backward compatible.
-def sha512_half(data: str) -> str: +def sha512_half(data: Union[str, bytes, bytearray, memoryview]) -> str: @@ - hash_obj = hashlib.sha512(bytes.fromhex(data)) + if isinstance(data, (bytes, bytearray, memoryview)): + buf = bytes(data) + elif isinstance(data, str): + buf = bytes.fromhex(data) + else: + raise TypeError("data must be hex str or bytes-like") + hash_obj = hashlib.sha512(buf) return hash_obj.hexdigest()[:64].upper()
58-73: Ruff TRY003: long exception message — add noqa to preserve test-coupled text.Tests assert on the message substring; instead of changing the text, silence TRY003 locally.
- if n < 0 or n > 0xFFFFFFFF: - raise ValueError("u32 sequence out of range (0..=2^32-1)") + if n < 0 or n > 0xFFFFFFFF: + raise ValueError("u32 sequence out of range (0..=2^32-1)") # noqa: TRY003
90-103: Avoid recomputing ord/format for ledger spaces.Precompute hex codes once; function remains the same API and keeps KeyError behavior.
LEDGER_SPACES = { @@ "depositPreauth": "p", } +# Precompute hex (4 chars) once at import time +LEDGER_SPACES_HEX = {k: format(ord(v), "x").zfill(4) for k, v in LEDGER_SPACES.items()} + @@ - return format(ord(LEDGER_SPACES[name]), "x").zfill(4) + return LEDGER_SPACES_HEX[name]
216-219: Minor: compare hex strings instead of big-int conversion.Both are fixed-length uppercase hex; lexicographic compare is equivalent and cheaper.
- if int(addr1_hex, 16) > int(addr2_hex, 16): + if addr1_hex > addr2_hex: addr1_hex, addr2_hex = addr2_hex, addr1_hex
220-232: Ruff TRY003 on explicit error messages — add noqa where needed.Keep the descriptive messages (tests assert on them) and suppress TRY003 inline.
- elif isinstance(currency, bytes): + elif isinstance(currency, bytes): if len(currency) != 20: - raise ValueError("Currency bytes must be exactly 20 bytes.") + raise ValueError("Currency bytes must be exactly 20 bytes.") # noqa: TRY003 currency_hex = currency.hex().upper() @@ - if not re.fullmatch(r"[0-9A-Fa-f]{40}", hex_str): - raise ValueError("Currency hex must be exactly 40 hex characters.") + if not re.fullmatch(r"[0-9A-Fa-f]{40}", hex_str): + raise ValueError("Currency hex must be exactly 40 hex characters.") # noqa: TRY003tests/unit/utils/test_hash_utils.py (2)
249-261: Cover invalid addresses across remaining hash functions.Extend negative tests to payment_channel, deposit_preauth, check, and ticket.
def test_invalid_address_in_hash_functions(self): """Test hash functions with invalid addresses.""" invalid_address = "invalid_address" with self.assertRaises((XRPLAddressCodecException, ValueError)): hash_account_root(invalid_address) with self.assertRaises((XRPLAddressCodecException, ValueError)): hash_offer(invalid_address, 123) with self.assertRaises((XRPLAddressCodecException, ValueError)): hash_escrow(invalid_address, 123) + + with self.assertRaises((XRPLAddressCodecException, ValueError)): + hash_payment_channel(invalid_address, "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", 1) + + with self.assertRaises((XRPLAddressCodecException, ValueError)): + hash_payment_channel("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", invalid_address, 1) + + with self.assertRaises((XRPLAddressCodecException, ValueError)): + hash_deposit_preauth(invalid_address, "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh") + + with self.assertRaises((XRPLAddressCodecException, ValueError)): + hash_deposit_preauth("rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh", invalid_address) + + with self.assertRaises((XRPLAddressCodecException, ValueError)): + hash_check(invalid_address, 1) + + with self.assertRaises((XRPLAddressCodecException, ValueError)): + hash_ticket(invalid_address, 1)
28-40: Add negative test for invalid hex to sha512_half.Ensures we raise cleanly on malformed input.
def test_sha512_half(self): """Test SHA512 half hash function.""" # Test with known input/output for "Hello World" test_data = "48656C6C6F20576F726C64" # "Hello World" in hex result = sha512_half(test_data) # Known SHA-512 half result for "Hello World" expected = "2C74FD17EDAFD80E8447B0D46741EE243B7EB74DD2149A0AB1B9246FB30382F2" self.assertEqual(result, expected) # Should be 64 characters (32 bytes) uppercase hex self.assertEqual(len(result), 64) self.assertTrue(result.isupper()) self.assertTrue(all(c in "0123456789ABCDEF" for c in result)) + # Invalid hex should raise + with self.assertRaises(ValueError): + sha512_half("ZZ") # non-hex + with self.assertRaises(ValueError): + sha512_half("A") # odd length
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/unit/utils/test_hash_utils.py(1 hunks)xrpl/utils/hash_utils.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/unit/utils/test_hash_utils.py (2)
xrpl/core/addresscodec/exceptions.py (1)
XRPLAddressCodecException(6-9)xrpl/utils/hash_utils.py (12)
address_to_hex(75-87)hash_account_root(105-114)hash_check(134-148)hash_deposit_preauth(255-269)hash_escrow(151-165)hash_offer(117-131)hash_payment_channel(168-184)hash_signer_list_id(187-198)hash_ticket(238-252)hash_trustline(201-235)ledger_space_hex(90-102)sha512_half(45-55)
xrpl/utils/hash_utils.py (2)
xrpl/wallet/main.py (1)
address(24-29)xrpl/core/addresscodec/codec.py (1)
decode_classic_address(146-156)
🪛 Ruff (0.12.2)
xrpl/utils/hash_utils.py
71-71: Avoid specifying long messages outside the exception class
(TRY003)
225-225: Avoid specifying long messages outside the exception class
(TRY003)
230-230: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
xrpl/utils/hash_utils.py (1)
1-42: Solid, spec-aligned implementation and API surface.The module structure, LEDGER_SPACES mapping, helpers, aliases, and docstrings look consistent with XRPL keylet/index derivations. Nice job keeping outputs uniformly 64-char uppercase hex and validating sequences.
tests/unit/utils/test_hash_utils.py (1)
328-343: LGTM on completeness checks.Good coverage of expected ledger spaces and hex formatting.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
xrpl/utils/hash_utils.py (1)
193-198: Payment channel ordering looks correct; please confirm with a fixed vectorCreator, destination, sequence ordering matches xrpl.js. Add/verify a test with a known vector to lock parity.
#!/bin/bash # Show payment-channel tests to confirm a fixed expected hash is asserted rg -nP -C3 '\bhash_payment_channel\(' tests | sed -n '1,200p'
🧹 Nitpick comments (2)
xrpl/utils/hash_utils.py (2)
68-69: Use digest() then hex() for SHA-512Half to avoid extra formatting workSmall perf win: avoid computing the full 128-hex hexdigest just to slice half.
- return hash_obj.hexdigest()[:64].upper() + return hash_obj.digest()[:32].hex().upper()
86-86: Normalize sequence hex to uppercase for consistencyAll other pieces are uppercase hex; make the sequence match.
- return format(n, "x").zfill(SEQ_HEX_LEN) + return f"{n:0{SEQ_HEX_LEN}X}"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/unit/utils/test_hash_utils.py(1 hunks)xrpl/utils/hash_utils.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/utils/test_hash_utils.py
🧰 Additional context used
🧬 Code graph analysis (1)
xrpl/utils/hash_utils.py (2)
xrpl/wallet/main.py (1)
address(24-29)xrpl/core/addresscodec/codec.py (1)
decode_classic_address(146-156)
🪛 Ruff (0.12.2)
xrpl/utils/hash_utils.py
66-66: Avoid specifying long messages outside the exception class
(TRY003)
239-239: Avoid specifying long messages outside the exception class
(TRY003)
244-244: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (7)
xrpl/utils/hash_utils.py (7)
101-101: LGTM: classic address decodingCorrect use of addresscodec; returns 20-byte AccountID and uppercases hex.
116-116: LGTM: precomputed ledger space hexKeys match xrpl.js keylet prefixes; 4-hex-digit format is correct.
141-145: LGTM: Offer index compositionNamespace + AccountID + u32(seq) matches OfferID derivation.
175-179: LGTM: Escrow index compositionMatches rippled/xrpl.js (u namespace + owner + u32(seq)).
210-212: LGTM: SignerList uses zeroed u32 suffixMatches SignerListID derivation.
230-233: LGTM: Trustline address orderingLower AccountID first matches canonical rippleState ordering.
285-287: LGTM: Aliases for OfferID and RippleStateGood for cross-lib parity.
…related comments for clarity
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/unit/utils/test_hash_utils.py (1)
132-143: Add a concrete payment-channel test vector for cross-lib parity.Currently only format is asserted. Please add an expected value from xrpl.js (same inputs) to lock ordering and compatibility.
Would you like me to fetch the xrpl.js vector and open a follow-up PR with the exact assertion?
🧹 Nitpick comments (3)
xrpl/utils/hash_utils.py (3)
61-69: Optional: silence TRY003 for the TypeError raise.Ruff flagged TRY003 earlier; other raises already carry
# noqa: TRY003. Consider matching here for consistency.Apply this minimal change:
- else: - raise TypeError("data must be hex str or bytes-like") + else: + raise TypeError("data must be hex str or bytes-like") # noqa: TRY003
72-87: Docstring/case mismatch in _u32_hex.Function returns uppercase hex (
X), but docstring claims lowercase. Update docstring to avoid confusion.- Returns: - 8-character lowercase hex string with zero padding. + Returns: + 8-character uppercase hex string with zero padding.
182-199: Payment channel input order matches XRPL (creator, destination, sequence).Implementation mirrors xrpl.js ordering. To prevent regressions, pair this with a known test vector (see test suggestion).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/unit/utils/test_hash_utils.py(1 hunks)xrpl/utils/hash_utils.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/unit/utils/test_hash_utils.py (2)
xrpl/core/addresscodec/exceptions.py (1)
XRPLAddressCodecException(6-9)xrpl/utils/hash_utils.py (11)
address_to_hex(89-101)hash_account_root(119-128)hash_check(148-162)hash_escrow(165-179)hash_offer(131-145)hash_payment_channel(182-198)hash_signer_list_id(201-212)hash_ticket(253-267)hash_trustline(215-250)ledger_space_hex(104-116)sha512_half(48-69)
xrpl/utils/hash_utils.py (1)
xrpl/core/addresscodec/codec.py (1)
decode_classic_address(146-156)
🪛 Ruff (0.12.2)
xrpl/utils/hash_utils.py
66-66: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (12)
tests/unit/utils/test_hash_utils.py (6)
28-55: Good coverage and vectors for sha512_half.Covers hex/bytes paths and invalid inputs thoroughly. LGTM.
70-79: Ledger-space hex expectations verified.Assertions match rippled/xrpl.js conventions (e.g., "0061" for account). LGTM.
96-112: Alias parity test is solid.hash_offer_id == hash_offer is well-validated. LGTM.
144-153: Trustline vector test is great.Validates address ordering and currency handling against known vector. LGTM.
301-333: Nice sweep of currency format cases.Covers 3-char, bytes(20), and 40-hex with case-canonicalization. LGTM.
354-369: Completeness check for LEDGER_SPACES is helpful.Ensures mapping remains comprehensive and hex-encodable. LGTM.
xrpl/utils/hash_utils.py (6)
44-46: Precomputed LEDGER_SPACES_HEX is correct and efficient.Mapping via ASCII code to 4-hex-digit namespace aligns with XRPL conventions. LGTM.
131-146: Offer hash construction looks correct.Namespace + account hex + u32 sequence (big-endian, zero-padded). LGTM.
201-213: SignerList index derivation is correct.Uses 0 as the discriminator after the owner account. LGTM.
215-251: Trustline: stable address ordering and strict currency validation look good.
- Addresses sorted by hex for determinism.
- 3-char codes canonicalized to uppercase with correct 20-byte packing.
- Bytes length and hex-length/charset validated with clear errors. LGTM.
253-268: Ticket: API and derivation align with XRPL (TicketSequence).Renamed parameter avoids confusion with on-ledger TicketID. LGTM.
270-285: DepositPreauth index derivation is correct and directional.Ordering matches spec (grantor first, authorized second). LGTM.
High Level Overview of Change
This PR implements ledger object hashing utilities for XRPL-py, providing functions to compute hashes of various XRPL ledger objects. These utilities are essential for Batch transactions where developers need to know the hash of a ledger entry object before it's created on the ledger.
Added Functions:
hash_account_root- Compute hash of AccountRoot ledger entrieshash_offer- Compute hash of Offer ledger entrieshash_escrow- Compute hash of Escrow ledger entrieshash_payment_channel- Compute hash of Payment Channel ledger entrieshash_trustline(alias:hash_ripple_state) - Compute hash of Trustline/RippleState ledger entrieshash_signer_list_id- Compute hash of SignerList ledger entrieshash_check- Compute hash of Check ledger entrieshash_ticket- Compute hash of Ticket ledger entrieshash_deposit_preauth- Compute hash of DepositPreauth ledger entriessha512_half,address_to_hex,ledger_space_hexContext of Change
As requested in the initial requirements: "Batch can sometimes require you to know the hash of the ledger entry object before processing therefore I would recommend we add the hashing functionality for all ledger objects into this repo so that developers don't need to build their own hashing functions."
This implementation follows the xrpl.js reference implementation to ensure compatibility and correctness. The architecture uses:
Alternative approaches considered:
Type of Change
Did you update CHANGELOG.md?
Test Plan
Test Coverage:
Manual Testing:
python -m pytest tests/unit/utils/test_hash_utils.py -vpython -m pytest tests/unit/utils/test_hash_utils.py --cov=xrpl.utils.hash_utils --cov-report=term-missingIntegration Testing: