Repository navigation
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes part 2 of #397 — datum hashing is now byte faithful. Part 1 is deliberately left out, with the reason below.
Data.toDatumHashhashed a re-encode of the decoded value. The ledger hashes a datum over the bytes it arrived in — cardano-ledger's PlutusDatakeeps the original memoized bytes — so a datum decoded from a valid but non-canonical encoding produced a hash the node never computed:Change
Following the pattern #397 itself points at — the WeakMap memo
Transactionalready uses for its format tree:Data.fromCBORBytes/Data.fromCBORHexremember a copy of the source bytes for the decoded datum, in a module-privateWeakMap, when decoding with the default codec options. Non-default options skip the cache, exactly asTransaction.fromCBORBytesdoes — explicit options signal the caller wants the re-encode.Data.toDatumHashunder the default options hashes the remembered bytes when there are any, and otherwise hashes the re-encode, as before.toCBORBytes/toCBORHexstill write the module's own encoding; nothing about encoding or decoding behaviour moves.Scope notes, both taken from the issue text:
bigint, which cannot be a WeakMap key — the issue calls this case out, and a test pins the fallback.{1: 1, 1: 2}as an inline datum — so rejecting duplicate keys on decode, the fail-closed option, would refuse valid on-chain datums, and preserving them needs theMapmodel itself to change. That is a design call I did not want to bury inside a hashing fix; the issue's own regression line allows either outcome ("rejected (or preserved, per chosen approach)"). Happy to take it up separately in whichever direction you prefer.Tests
Ten new cases in
test/Data.DatumHashOriginalBytes.test.ts, each checked against main first:fromCBORBytes— RED on main, wheretoDatumHashreturned the hash of the re-encode;Results: the new file passes 10/10; the Data and hashing files around it pass (
Data116,Data.encoder44,Data.golden30,Data.prop13,CBOR.PlutusDataPresets27,UtilsHash.CML14,TransactionWitnessSet.DataOptions118). The full package suite passes: 1688 passed / 75 skipped / 0 failed, test files 102 passed / 1 skipped of 103 — exactly main's 1678 baseline + 10 new, files +1.tsc --noEmitand eslint are clean, andData.ts's prettier diff count matches main's exactly (7 = 7). A patch changeset is included.Independent builder — not affiliated with IntersectMBO, the Cardano Foundation or IOG.
@Kshot9000 (https://x.com/kshot9000) · ADA: addr1q8hnl6vl5a6k3rw3n5g3jtte696zcl76kfatzv7gpswa9r0dj7fma6klq55y4ffm7tf0em09udnyhuk4ah92pl5x9jpqjae44v
cc @cardano-foundation @IntersectMBO